Skip to content

Conversation

@alvin-r
Copy link
Contributor

@alvin-r alvin-r commented Mar 13, 2025

User description

…ng found as a helper of helper. This resulted in missing context for the LLM.

bug resulted in functions being redefined: alvin-r/ultralytics#4
after the fix: alvin-r/ultralytics#7


PR Type

  • Bug fix
  • Tests

Description

  • Fix context extraction by including the target function as a helper.

  • Automatically add init methods to helper qualified names.

  • Add a new test verifying indirect init helper detection.


Changes walkthrough 📝

Relevant files
Bug fix
code_context_extractor.py
Update context extraction with __init__ helper handling. 

codeflash/context/code_context_extractor.py

  • Insert function-to-optimize as a helper early in processing.
  • Auto-append __init__ method names to helper qualified names.
  • Reorder helper extraction to process __init__ methods correctly.
  • +11/-4   
    Tests
    test_code_context_extractor.py
    Add test for indirect __init__ helper context extraction.

    tests/test_code_context_extractor.py

  • Add test_indirect_init_helper to validate __init__ inclusion.
  • Ensure indirect helper functions are correctly recognized.
  • +54/-1   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Indentation

    Review the updated loop that supplements helper qualified names with init variants. Ensure that the indentation (notably in the code block updating the set of qualified names) is consistent with project style guidelines.

    for qualified_names in helpers_of_fto_qualified_names_dict.values():
         qualified_names.update({f"{qn.rsplit('.', 1)[0]}.__init__" for qn in qualified_names if '.' in qn})
    Fragile Test

    The new test for indirect init helper detection compares multiline string contexts. Verify that minor formatting adjustments or whitespace differences will not cause false negatives in the test.

    def test_indirect_init_helper() -> None:
        code = """
    class MyClass:
        def __init__(self):
            self.x = 1
            self.y = outside_method()
        def target_method(self):
            return self.x + self.y
    
    def outside_method():
        return 1
    """
        with tempfile.NamedTemporaryFile(mode="w") as f:
            f.write(code)
            f.flush()
            file_path = Path(f.name).resolve()
            opt = Optimizer(
                Namespace(
                    project_root=file_path.parent.resolve(),
                    disable_telemetry=True,
                    tests_root="tests",
                    test_framework="pytest",
                    pytest_cmd="pytest",
                    experiment_id=None,
                    test_project_root=Path().resolve(),
                )
            )
            function_to_optimize = FunctionToOptimize(
                function_name="target_method",
                file_path=file_path,
                parents=[FunctionParent(name="MyClass", type="ClassDef")],
                starting_line=None,
                ending_line=None,
            )
    
            code_ctx = get_code_optimization_context(function_to_optimize, opt.args.project_root)
            read_write_context, read_only_context = code_ctx.read_writable_code, code_ctx.read_only_context_code
            expected_read_write_context = """
    class MyClass:
        def __init__(self):
            self.x = 1
            self.y = outside_method()
        def target_method(self):
            return self.x + self.y
    """
            expected_read_only_context = f"""
    ```python:{file_path.relative_to(opt.args.project_root)}
    def outside_method():
        return 1

    """
    assert read_write_context.strip() == expected_read_write_context.strip()
    assert read_only_context.strip() == expected_read_only_context.strip()

    
    </details>
    
    </td></tr>
    </table>
    

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Verify dictionary key existence

    Ensure the dictionary key exists before adding to avoid runtime errors.

    codeflash/context/code_context_extractor.py [35-36]

     fto_as_function_source = get_function_to_optimize_as_function_source(function_to_optimize, project_root_path)
    -helpers_of_fto_dict[function_to_optimize.file_path].add(fto_as_function_source)
    +helpers_of_fto_dict.setdefault(function_to_optimize.file_path, set()).add(fto_as_function_source)
    Suggestion importance[1-10]: 8

    __

    Why: This suggestion introduces a defensive check using setdefault, which can prevent potential runtime errors if the expected dictionary key is missing. It correctly targets the added code lines and offers a valid, impactful improvement.

    Medium

    @alvin-r alvin-r requested review from KRRT7 and misrasaurabh1 March 13, 2025 18:46
    @alvin-r alvin-r merged commit a651fbf into main Mar 14, 2025
    16 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    3 participants